Skip to content

Fix instrument memory leaks#32721

Open
CubikingChill wants to merge 1 commit intomusescore:masterfrom
CubikingChill:fix-instrument-memory-leaks
Open

Fix instrument memory leaks#32721
CubikingChill wants to merge 1 commit intomusescore:masterfrom
CubikingChill:fix-instrument-memory-leaks

Conversation

@CubikingChill
Copy link
Contributor

@CubikingChill CubikingChill commented Mar 21, 2026

Resolves: #32722

  • I signed the CLA
  • The title of the PR describes the problem it addresses
  • Each commit's message describes its purpose and effects, and references the issue it resolves
  • If changes are extensive, there is a sequence of easily reviewable commits
  • The code in the PR follows the coding rules
  • There are no unnecessary changes
  • The code compiles and runs on my machine, preferably after each commit individually
  • I created a unit test or vtest to verify the changes I made (if applicable)

@CubikingChill CubikingChill force-pushed the fix-instrument-memory-leaks branch 3 times, most recently from b154a70 to 4f2280e Compare March 21, 2026 01:05
void setInstrument(Instrument* i)
{
if (m_instrument != i) {
delete m_instrument;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would be better to change it to share_ptr?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume it would be a more significant change. Maybe it will be the next PR after this is merged.

@CubikingChill CubikingChill force-pushed the fix-instrument-memory-leaks branch from 4f2280e to 770f22b Compare March 24, 2026 16:28
@CubikingChill CubikingChill force-pushed the fix-instrument-memory-leaks branch from 770f22b to dd147c7 Compare March 24, 2026 20:24
Multiple memory leaks were present in the instrument ownership model:

1. Part destructor: Added destructor to delete all instruments in
   m_instruments map when Part is destroyed.

2. InstrumentList::setInstrument: Now deletes old instrument pointer
   before replacing it when the same tick already has an instrument.

3. Part::setInstruments: Now deletes all old instruments before
   clearing the map and setting new ones.

4. Part::removeInstrument: Now deletes instrument before erasing from map
   when InstrumentChange elements are removed.

5. Part::removeNonPrimaryInstruments: Now deletes instruments before
   erasing when removing non-primary instruments.

6. InstrumentChange::setInstrument(Instrument*): Now deletes old
   instrument pointer before assigning new one (with self-assignment check).

7. ChangeInstrument::cleanup: Added cleanup method to delete the
   unused instrument pointer after undo/redo operations.

These fixes ensure proper memory management throughout the instrument
lifecycle and prevent leaks during instrument changes, undo/redo, and
Part destruction.
@CubikingChill CubikingChill reopened this Mar 24, 2026
@CubikingChill CubikingChill force-pushed the fix-instrument-memory-leaks branch from dd147c7 to 722557a Compare March 24, 2026 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Memory Leaks in Instrument Management System

2 participants